Skip to content

Conversation

@Rexicon226
Copy link
Contributor

It's preferred to use the random seed provided by the stdlib (which changes every run, but is deterministic, and can be passed into the test runner again to reproduce identical results), instead of just hardcoding a single number. This ensures the unit doesn't have some hidden dependence on the seed of the prng used.

Had to fix a couple cases just like this, where we would hardcode expected results which depend on the seed to the prng. The change makes it clearer what inputs are used to get said output.

Almost the entire diff is just that, maybe a few little fixes in random places.

@github-project-automation github-project-automation bot moved this to 🏗 In progress in Sig Oct 4, 2025
@Rexicon226 Rexicon226 force-pushed the Rexicon226/random-seed branch from c189c0a to 677c7ca Compare October 4, 2025 20:54
@codecov
Copy link

codecov bot commented Oct 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/accountsdb/buffer_pool.zig 95.09% <100.00%> (ø)
src/accountsdb/db.zig 89.58% <100.00%> (ø)
src/accountsdb/manager.zig 97.09% <100.00%> (ø)
src/accountsdb/snapshot/download.zig 76.94% <100.00%> (ø)
src/accountsdb/swiss_map.zig 97.04% <100.00%> (ø)
src/consensus/fork_choice.zig 98.12% <100.00%> (ø)
src/consensus/latest_validator_votes.zig 95.37% <100.00%> (ø)
src/consensus/replay_tower.zig 93.68% <100.00%> (ø)
src/consensus/vote_listener.zig 93.46% <100.00%> (ø)
src/consensus/vote_tracker.zig 75.16% <100.00%> (ø)
... and 73 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@dnut dnut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I like this change. I commented on some stylistic preferences but I'm not asking you to change those things. My requested changes are:

  • remove the TODO
  • either remove // sig fmt: off and // sig fmt: on or consolidate it back to one line
  • restore the . or find another solution for the problem it was supposed to solve

@github-project-automation github-project-automation bot moved this from 🏗 In progress to 👀 In review in Sig Oct 6, 2025
dnut
dnut previously approved these changes Oct 7, 2025
Copy link
Contributor

@dnut dnut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to resolve all my conversations in the other review after responding to my questions.

yewman
yewman previously approved these changes Oct 21, 2025
@Rexicon226 Rexicon226 dismissed stale reviews from yewman and dnut via d4350cd October 28, 2025 12:42
@Rexicon226 Rexicon226 force-pushed the Rexicon226/random-seed branch from 5e86bbb to d4350cd Compare October 28, 2025 12:42
@Rexicon226 Rexicon226 force-pushed the Rexicon226/random-seed branch from 0a71dd5 to f222f2d Compare October 31, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

4 participants